Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RNET-1161: Implement support for using a log level for a specific log category #3634

Merged
merged 44 commits into from
Jul 12, 2024

Conversation

elle-j
Copy link
Contributor

@elle-j elle-j commented Jun 26, 2024

Description

Deprecates Logger in favor of its new base class RealmLogger (see notes below on avoiding breaking changes).

Primarily, enables more control over which category of messages should be logged and at which criticality level, by setting and getting a LogLevel for a given LogCategory. The hierarchy of categories starts at LogCategory.Realm.

RealmLogger.SetLogLevel(LogLevel.Warn, LogCategory.Realm.Sync);
RealmLogger.GetLogLevel(LogCategory.Realm.Sync.Client.Session); // LogLevel.Warn

Adds a function logger that accepts a callback that will receive the LogLevel, LogCategory, and the message when invoked.

RealmLogger.Default = RealmLogger.Function((level, category, message) => /* custom implementation */);

Adds a RealmLogger.Log() overload taking a category.

RealmLogger.Default.Log(LogLevel.Warn, LogCategory.Realm, "A warning message");

Notes on avoiding breaking changes

Would-be breaking changes

Users have been able to provide their own Logger subclass via the Logger.Default set accessor. Custom loggers need to override and implement Logger.LogImpl(), and with the addition of log categories in this PR, the LogImpl() signature would require breaking changes.

Solution

Essentially, the previous Logger class has been renamed RealmLogger, and another class called Logger now derives from RealmLogger, with the only own member being the LogImpl() with an abstract method identical to before to avoid the breaking change.

public abstract class Logger : RealmLogger
{
    protected abstract void LogImpl(LogLevel level, string message);

    protected override void LogImpl(LogLevel level, LogCategory category, string message)
    {
        LogImpl(level, message);
    }
}

Fixes #3633

@realm/devdocs

TODO

  • Changelog entry
  • Tests
  • API docs

Copy link

coveralls-official bot commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9679342781

Details

  • 77 of 80 (96.25%) changed or added relevant lines in 3 files are covered.
  • 146 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.02%) to 81.337%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Logging/Logger.cs 26 29 89.66%
Files with Coverage Reduction New Missed Lines %
Realm/Realm/DatabaseTypes/QueryArgument.cs 1 89.71%
Realm/Realm/Handles/SessionHandle.cs 7 84.55%
Realm/Realm/Native/PrimitiveValue.cs 11 81.92%
Realm/Realm/Native/SyncSocketProvider.cs 25 0.0%
Realm/Realm/Native/SyncSocketProvider.EventLoop.cs 31 0.0%
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 71 0.0%
Totals Coverage Status
Change from base Build 9661237267: -0.02%
Covered Lines: 6889
Relevant Lines: 8322

💛 - Coveralls

Copy link

coveralls-official bot commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9681475484

Details

  • 77 of 80 (96.25%) changed or added relevant lines in 3 files are covered.
  • 146 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.004%) to 81.354%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Logging/Logger.cs 26 29 89.66%
Files with Coverage Reduction New Missed Lines %
Realm/Realm/DatabaseTypes/QueryArgument.cs 1 89.71%
Realm/Realm/Handles/SessionHandle.cs 7 84.55%
Realm/Realm/Native/PrimitiveValue.cs 11 81.92%
Realm/Realm/Native/SyncSocketProvider.cs 25 0.0%
Realm/Realm/Native/SyncSocketProvider.EventLoop.cs 31 0.0%
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 71 0.0%
Totals Coverage Status
Change from base Build 9661237267: -0.004%
Covered Lines: 6890
Relevant Lines: 8322

💛 - Coveralls

Realm/Realm/Logging/LogCategory.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
wrappers/src/shared_realm_cs.cpp Outdated Show resolved Hide resolved
Realm/Realm/Logging/LogCategory.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Show resolved Hide resolved
Copy link

coveralls-official bot commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9715427241

Details

  • 73 of 75 (97.33%) changed or added relevant lines in 3 files are covered.
  • 146 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.005%) to 81.353%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Logging/Logger.cs 24 26 92.31%
Files with Coverage Reduction New Missed Lines %
Realm/Realm/DatabaseTypes/QueryArgument.cs 1 89.71%
Realm/Realm/Handles/SessionHandle.cs 7 84.55%
Realm/Realm/Native/PrimitiveValue.cs 11 81.92%
Realm/Realm/Native/SyncSocketProvider.cs 25 0.0%
Realm/Realm/Native/SyncSocketProvider.EventLoop.cs 31 0.0%
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 71 0.0%
Totals Coverage Status
Change from base Build 9661237267: -0.005%
Covered Lines: 6885
Relevant Lines: 8316

💛 - Coveralls

Copy link

coveralls-official bot commented Jul 1, 2024

Pull Request Test Coverage Report for Build 9742115832

Details

  • 87 of 89 (97.75%) changed or added relevant lines in 3 files are covered.
  • 146 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.02%) to 81.38%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Logging/Logger.cs 24 26 92.31%
Files with Coverage Reduction New Missed Lines %
Realm/Realm/DatabaseTypes/QueryArgument.cs 1 89.71%
Realm/Realm/Handles/SessionHandle.cs 7 84.55%
Realm/Realm/Native/PrimitiveValue.cs 11 81.92%
Realm/Realm/Native/SyncSocketProvider.cs 25 0.0%
Realm/Realm/Native/SyncSocketProvider.EventLoop.cs 31 0.0%
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 71 0.0%
Totals Coverage Status
Change from base Build 9661237267: 0.02%
Covered Lines: 6899
Relevant Lines: 8330

💛 - Coveralls

@elle-j elle-j marked this pull request as ready for review July 2, 2024 17:11
@elle-j elle-j requested review from clementetb, papafe and nirinchev July 2, 2024 17:22
Realm/Realm.UnityUtils/UnityLogger.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/LogCategory.cs Outdated Show resolved Hide resolved
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs Outdated Show resolved Hide resolved
wrappers/src/shared_realm_cs.cpp Outdated Show resolved Hide resolved
Copy link

coveralls-official bot commented Jul 5, 2024

Pull Request Test Coverage Report for Build 9810938552

Details

  • 89 of 93 (95.7%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 81.489%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 0 1 0.0%
Realm/Realm/Logging/Logger.cs 22 25 88.0%
Totals Coverage Status
Change from base Build 9795389719: 0.1%
Covered Lines: 6903
Relevant Lines: 8317

💛 - Coveralls

Realm/Realm/Handles/SharedRealmHandle.cs Show resolved Hide resolved
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Outdated Show resolved Hide resolved
wrappers/src/marshalling.hpp Outdated Show resolved Hide resolved
wrappers/src/marshalling.hpp Outdated Show resolved Hide resolved
Copy link

coveralls-official bot commented Jul 10, 2024

Pull Request Test Coverage Report for Build 9894480239

Details

  • 102 of 133 (76.69%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 81.499%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Realm/Realm/Handles/RealmHandle.cs 0 1 0.0%
Realm/Realm/Handles/SyncUserHandle.cs 0 1 0.0%
Realm/Realm/Helpers/Argument.cs 0 1 0.0%
Realm/Realm/Handles/SessionHandle.cs 2 4 50.0%
Realm/Realm/Native/SyncSocketProvider.cs 0 2 0.0%
Realm/Realm/Logging/Logger.cs 30 33 90.91%
Realm/Realm/Native/NativeCommon.cs 0 3 0.0%
Realm/Realm/Native/SyncSocketProvider.EventLoop.cs 0 9 0.0%
Realm/Realm/Native/SyncSocketProvider.WebSocket.cs 0 9 0.0%
Totals Coverage Status
Change from base Build 9865287337: 0.1%
Covered Lines: 6908
Relevant Lines: 8322

💛 - Coveralls

@@ -223,8 +236,14 @@ private static class NativeMethods
[DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_refresh_async", CallingConvention = CallingConvention.Cdecl)]
public static extern bool refresh_async(SharedRealmHandle realm, IntPtr tcs_handle, out NativeException ex);

[DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_get_log_level", CallingConvention = CallingConvention.Cdecl)]
public static extern LogLevel get_log_level([MarshalAs(UnmanagedType.LPWStr)] string category_name, IntPtr category_name_len);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're trying to go with using Realm.Native.StringValue in wrapper calls when needing to pass strings. Am I correct @nirinchev ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding here was that StringValue is mainly used for the returned strings (Core -> DotNet), whereas the MarshalAs is DotNet -> Core primarily?

Realm/Realm/Logging/LogCategory.cs Show resolved Hide resolved
Realm/Realm/Logging/LogCategory.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Show resolved Hide resolved
@@ -20,11 +20,11 @@

namespace UnityUtils
{
public class UnityLogger : Logger
public class UnityLogger : RealmLogger

Check notice

Code scanning / CodeQL

Missing a summary in documentation comment Note

Documentation should have a summary.
Realm/Realm/Logging/Logger.cs Dismissed Show dismissed Hide dismissed
Realm/Realm/Helpers/Argument.cs Outdated Show resolved Hide resolved
Realm/Realm/Logging/Logger.cs Show resolved Hide resolved
@elle-j elle-j merged commit 84940b3 into main Jul 12, 2024
56 of 60 checks passed
@elle-j elle-j deleted the lj/logger branch July 12, 2024 09:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support using a log level for a specific log category
4 participants